Add Comprehensive Test for Multigeometry Range Classes #1197
Conversation
github-actions
bot
added
cmake
This reverts commit b4a2421.
…into techdebt/test_range_ref
isVoid
added
tech debt
| @@ -59,13 +59,13 @@ CUSPATIAL_HOST_DEVICE auto polygon_ref<RingIterator, VecIterator>::ring_end() co | |||
| template <typename RingIterator, typename VecIterator> | |||
| CUSPATIAL_HOST_DEVICE auto polygon_ref<RingIterator, VecIterator>::point_begin() const | |||
| { | |||
| return _point_begin; | |||
| return thrust::next(_point_begin, *_ring_begin); | |||
There was a problem hiding this comment.
Can you explain these changes? Why can't point_begin be used as stored?
There was a problem hiding this comment.
tl;dr. Because point_begin API is supposed to return only the point range to the current multilinestring (not all multilinestrings). And that the offsets stored in a *_ref are global offsets to the lower level range.
Consider this example:
Multilinestring_range:
Geometry offsets: [0, 2, 4]
Part Offsets: [0, 2, 4, 6, 8]
Points: [P0, P1... P8]
A multilinestring_ref constructed for the second multilinestring is stored as below:
Part Offsets: [4, 6, 8]
Points: [P0, ... P8]
Notice that in part offset, the offsets are global offsets (4 means that the first point to the multilinestring locates at _point_begin + 4). You may ask why we don't store as below:
Part Offsets: [0, 2, 4]
Points: [P4 ... P8]
That's an alternative too. Just different ways to skin a cat. I don't see an advantage to either one.
| } | ||
|
|
||
| template <typename PartIterator, typename VecIterator> | ||
| CUSPATIAL_HOST_DEVICE auto multilinestring_ref<PartIterator, VecIterator>::point_end() const | ||
| { | ||
| return _point_end; | ||
| // _part_end is the one-past the last part index to the point of this multilinestring. |
There was a problem hiding this comment.
| // _part_end is the one-past the last part index to the point of this multilinestring. | |
| // _part_end refers to one past the last part index to the points of this multilinestring. |
I don't quite understand this. A part index indexes points? Is it one past the last point of the part?
There was a problem hiding this comment.
See how multilinestring_ref is constructed:
*(_part_begin + _geometry_begin[i + 1]) is the part index of the last point of the current multilinestring. When constructing it, I increment it to maintain that the the end interator of a range should be one-past the last element.
… into techdebt/test_range_ref
|
/merge |
Description
closes #991
This PR adds comprehensive tests for
multi*_rangeclass. The basic goal is to make sure every API in service is at least covered by 3 tests: a range with 0, 1 and 1000 elements. The tests are structured as below: for each range, a base fixture class is created. The base class defines virtual functions that its subclass should implement. These virtual functions include amake_test_multi*function that constructs the geometry array of that specific test case, as well as each sub test function that defines the expected value of that test case. The base class defines aSetUpfunction that calls themake_test_multi*to generate the array at test start up time. Then, arun_testfunction is defined to subsequently call every sub routine that test every API of that geometry range.In addition, this PR fixes several small bug in
point_beginaccessor inlinestring_refandmultipolygon_refclass. Also, a few unused functions inmultipolygon_rangehave been removed.Checklist